Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass kwargs to pool constructor in choose_pool #4949

Merged
merged 2 commits into from
Jan 13, 2025

Conversation

mj-will
Copy link
Contributor

@mj-will mj-will commented Nov 20, 2024

Pass keyword arguments passed to choose_pool to the pool constructor. This enables additional configuration of the pool.

Standard information about the request

This is a: bug fix

This change affects: pycbc_bruke_bank, inference. However, since choose_pool is never called with any keyword arguments, things are functionally the same.

This change changes: configuration

This change: has appropriate unit tests, follows style guidelines (See e.g. PEP8)

This change is backwards compatible

Motivation

This enables specifying e.g. initalizer and initargs when creating a pool. Both the default multiprocessing library and schwimmbad support these arguments.

This will allow me to improve how the multiprocessing pool is configured for nessai, which will be a subsequent PR.

Contents

I've added kwargs to choose_pool. I've also updated SinglePool to allow arbitrary keyword arguments, this is consistent with SerialPool from schwimmbad.

Links to any issues or associated PRs

N/A

Testing performed

The changes are covered by existing tests.

Additional notes

  • The author of this pull request confirms they will adhere to the code of conduct

Copy link
Contributor

@GarethCabournDavies GarethCabournDavies left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a reason not to approve this

@mj-will
Copy link
Contributor Author

mj-will commented Jan 13, 2025

I don't see a reason not to approve this

Thanks Gareth, any suggestions for who else I should get to look at this? (If that's needed)

@GarethCabournDavies
Copy link
Contributor

its a small enough change that i don't think its needed

Copy link
Member

@ahnitz ahnitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine to me

@ahnitz ahnitz merged commit 0dfb476 into gwastro:master Jan 13, 2025
29 checks passed
@mj-will mj-will deleted the choose-pool-kwargs branch January 13, 2025 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants